Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

teec: do fail on MAX_SIZE allocation requests #372

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

ldts
Copy link
Contributor

@ldts ldts commented Jan 16, 2024

The variable aligned_sz will be 0 when the requested sz is MAX_SIZE. Since posix_memalign can return a valid pointer for zero size allocations, share memory registration requests for MAX_SIZE might make it to the kernel.

This PR stops it early - just as it was before "teec: use multiple of page size for page aligned buffers" was merged.

Fixes: acb0885 ("teec: use multiple of page size for page aligned buffers")

@@ -82,7 +82,8 @@ static void *teec_paged_aligned_alloc(size_t sz)
size_t page_sz = sysconf(_SC_PAGESIZE);
size_t aligned_sz = ((sz + page_sz - 1) / page_sz) * page_sz;

if (!posix_memalign(&p, page_sz, aligned_sz))
/* aligned_sz will be null if MAX_SIZE was requested */
if (aligned_sz && !posix_memalign(&p, page_sz, aligned_sz))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about doing a classic overflow check like:

aligned_sz >= sz && ...

to make it more obvious.

Copy link
Contributor Author

@ldts ldts Jan 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I will then remove the comment .

This should work just because TEEC_RegisterSharedMemory checks for size 0 and requests 8 bytes instead btw. Otherwise we would be calling the kernel unnecessarily since the aligned_size would be 0. maybe something to remember.

@jenswi-linaro
Copy link
Contributor

Reviewed-by: Jens Wiklander <[email protected]>

@ldts
Copy link
Contributor Author

ldts commented Jan 16, 2024

tags added and versal retested

The variable aligned_sz will be 0 when the requested sz is MAX_SIZE.
Since posix_memalign can return a valid pointer for zero size
allocations, share memory registration requests for MAX_SIZE might make
it to the kernel.

This PR stops it early - just as it was before "teec: use multiple of
page size for page aligned buffers" was merged.

Fixes: acb0885 ("teec: use multiple of page size for page aligned buffers")
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
@ldts
Copy link
Contributor Author

ldts commented Jan 16, 2024

had to fix the broken reference to the "Fixes" commit. all good now.

@jforissier jforissier merged commit f7e4ced into OP-TEE:master Jan 16, 2024
3 checks passed
@ldts ldts deleted the allocate branch January 16, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants